Skip to content

Mcp.core.distributed#1367

Open
xiangyan99 wants to merge 5 commits intomodelcontextprotocol:mainfrom
xiangyan99:mcp.core.distributed
Open

Mcp.core.distributed#1367
xiangyan99 wants to merge 5 commits intomodelcontextprotocol:mainfrom
xiangyan99:mcp.core.distributed

Conversation

@xiangyan99
Copy link

Motivation and Context

Added ModelContextProtocol.AspNetCore.Distributed library

#828

How Has This Been Tested?

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@xiangyan99 xiangyan99 marked this pull request as ready for review February 24, 2026 16:55
@halter73 halter73 self-assigned this Feb 26, 2026
/// </remarks>
internal sealed class HybridCacheSessionStore : ISessionStore
{
private static readonly TimeSpan DefaultSessionTimeout = TimeSpan.FromMinutes(15);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is way too short. Please change to at least double that.

### Minimal Redis Configuration

```csharp
using Azure.Identity;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These docs are a bit too azure-y. They should be kept generic. Also, they are missing the WithSessionAffinity call.

<PackageVersion Include="System.Linq.AsyncEnumerable" Version="$(System10Version)" />
<PackageVersion Include="xunit.v3" Version="3.2.2" />
<PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
<PackageVersion Include="NSubstitute" Version="5.3.0" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They already have moq in this repo. I wouldn't be surprised if they wanted us to keep using that instead of NSubstitute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants